SHIP-0038: Add shipwright-build-strategy-controller #169
Conversation
pull-request-size
bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jkhelil
force-pushed
the
shipwright-build-strategy-controller
branch
from
e4d727b to
5ab6c77
Compare
SaschaSchwarze0
changed the title
|
/cc @adambkaplan |
|
|
||
| ```yaml | ||
| apiVersion: operator.shipwright.io/v1alpha1 | ||
| kind: ShipwrightBuildStrategy |
There was a problem hiding this comment.
Do we really need a new CRD? Can't we extend the existing ShipwrightBuild CRD instead?
There was a problem hiding this comment.
Initially I was thinking about avoiding concurrency issues when updating the CR status, but now that you raise the comment and after thinking about it, We do not need a new CRD, as the controller design is leveraging the optimistic concurrency and if the update fails, we should requeue
I will update the SHIP using the same CRD ShipwrightBuild
| ```go | ||
| main.go | ||
| controllers | ||
| controller.go | ||
| add_shipwrightbuild.go | ||
| add_shipwrightbuildstrategy.go | ||
| shipwrightbuild | ||
| shipwrightbuild_controller.go | ||
| shipwrightbuildstragegy | ||
| shipwrightbuildstrategy_controller.go | ||
| ``` |
There was a problem hiding this comment.
This feels like deeper implementation details - we don't want to be overly prescriptive in the proposal when it comes to how this functionality is delivered.
There was a problem hiding this comment.
I will update the SHIP and add only requirement or high level design recommendation
We do want One single manager and multiple controllers assigned to it, which means e require a new controller for installing the strategies
| The default strategy catalog or any url provided catalog can be unreachable if the kubernetes cluster needs | ||
| - a proxy to access to the catalog url | ||
| - it is a disconnected cluster and doesnt have any access to internet |
There was a problem hiding this comment.
Long term, do we need to think about providing OCI artifact resolvers, like Tekton does with task catalogs? Mirroring images is a well established practice for network-constrained environments.
| - "@qu1queee" | ||
| creation-date: 2023-10-02 | ||
| last-updated: 2023-10-02 | ||
| status: implementable |
There was a problem hiding this comment.
@qu1queee for "implementable", should we require that the drawbacks and alternatives sections be filled in? These sections aren't marked as required in the current template.
jkhelil
changed the title
jkhelil
force-pushed
the
shipwright-build-strategy-controller
branch
3 times, most recently
from
80d4b77 to
636eaa4
Compare
jkhelil
force-pushed
the
shipwright-build-strategy-controller
branch
from
636eaa4 to
b05efc2
Compare
jkhelil
force-pushed
the
shipwright-build-strategy-controller
branch
from
b05efc2 to
55f04dd
Compare
Add SHIP to propose the shipwright-build-strategy-controller